Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor codebase into installable "reprostim" Python module (+ reprostim-videocapture) package #124

Merged
merged 51 commits into from
Jan 22, 2025

Conversation

yarikoptic
Copy link
Member

@yarikoptic yarikoptic commented Dec 11, 2024

ref:

Notes.

  • Massive move of files was done in a commit directly in master to minimize size of this PR. Original commit 4c59d48 was cherry picked as 98677dd .

Initial scripts to expose as entry points

  • Parsing/parse_wQR.py : reprostim qr-parse
  • src/reprostim-capture/nosignal/reprostim/nosignal : reprostim detect-noscreen
  • tools/reprostim-timesync-stimuli: reprostim timesync-stimuli
    • to make it output --help quickly etc, delay import of psychopy etc until used in the function
  • fix reprostim-capture C/C++ CI workflow action failure caused by catch2 library v2->v3 external upgrade

@yarikoptic yarikoptic changed the title Initial doc sketch for how to RF codebase Refactoring of the codebase into "reprostim" Python module (+ reprostim-videocapture) package Dec 13, 2024
vmdocua added a commit that referenced this pull request Dec 16, 2024
vmdocua added a commit that referenced this pull request Dec 16, 2024
vmdocua added a commit that referenced this pull request Dec 16, 2024
vmdocua added a commit that referenced this pull request Dec 16, 2024
vmdocua added a commit that referenced this pull request Dec 16, 2024
vmdocua added a commit that referenced this pull request Dec 16, 2024
vmdocua added a commit that referenced this pull request Dec 16, 2024
pyproject.toml Outdated Show resolved Hide resolved
"Programming Language :: Python :: Implementation :: PyPy",
]
dependencies = []

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

define extras for psychopy and all so by default psychopy is not installed.

Copy link
Contributor

@candleindark candleindark Jan 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is resolved. ⬆️

vmdocua added a commit that referenced this pull request Dec 18, 2024
vmdocua added a commit that referenced this pull request Dec 18, 2024
vmdocua added a commit that referenced this pull request Dec 18, 2024
@yarikoptic
Copy link
Member Author

@candleindark please review this PR.

pyproject.toml Outdated
"pyaudio>=0.2.14",
"reedsolo>=1.7.0",
"psychopy",
"psychopy-sounddevice",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note: bummer can't reuse other definitions like in setup.cfg

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It can be done differently though. See comment below.

]

[project.optional-dependencies]
test = [
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of specifying an optional dependency for testing. It would be better if a test environment is specified with the same dependency. See https://hatch.pypa.io/latest/config/environment/overview/#dependencies for details.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May be I'll need is some details, because initially thought about this like we always have default "dependencies" and in case of optional "test" one it will add something specific to this area, like "pytest". But I can be wrong in my assumptions.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is nothing wrong with your current setup. However, the current setup is not taking advantage of Hatch's ability to manage environments for you.

Strictly speaking, test is not really a feature of the app like audio and psychopy. It is more of an environment you that want to set up to run tests of the app. Hatch allows you to define different environments, with different dependencies, to do different things. For example, you can set up environments for static type checking, linting, and testing. All environments have the required dependencies of the project installed, and you can specified additional dependencies just for a particular environment. Run hatch env show you will see all the available environments except the internal ones. Run hatch env show -i you will see the internal environments use by some of the Hatch subcommands such as hatch fmt.

My point is that instead of defining a test feature/extra, you may want to define one or multiple test environments. With environment definitions, you can have fine control of many aspect of the environments. For examples, you can create an environment just with the required dependencies installed, an environment with a feature dependency install, and run different tests in different environments. I have created a test environment for the dandisets-linkml-status-tools project and use it for testing. Additionally, hatch comes with a hatch-test internal environment which you can customize for your need.

pyproject.toml Outdated
classifiers = [
"Development Status :: 3 - Alpha",
"Programming Language :: Python",
"Programming Language :: Python :: 3.8",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we still supporting python 3.8?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed older Python versions before 3.10. Actually the lowest python version "denominator" is mostly defined by psychopy. While it can run under Python 3.8, the 3.10 version is suggested - in my experiments with Linux/MacOS only Python 3.10 worked well. So let's stick to it ATM. May be with time we can allow other implementations on demand.

pyproject.toml Outdated Show resolved Hide resolved
@click.option(
"-l",
"--log-level",
default="DEBUG",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't "INFO" a more common default for --log-level?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Setting default log level to "DEBUG" seems a bit excessive to me. For example, the logs clutter simple usages as shown below.

▶ reprostim qr-parse --help
2025-01-17 13:42:23,246 [DEBUG] Logging level set to: DEBUG
2025-01-17 13:42:23,246 [DEBUG] reprostim v0.0.6
2025-01-17 13:42:23,246 [DEBUG] main(...), command=qr-parse
Usage: reprostim qr-parse [OPTIONS] PATH

  Utility to parse video and locate integrated QR time codes.

Options:
  --mode [PARSE|INFO]  Specify execution mode. Default is "PARSE", normal
                       execution. Use "INFO" to dump video file info like
                       duration, bitrate, file size etc, (in this case "path"
                       argument specifies video file or directory containing
                       video files).
  --help               Show this message and exit.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it's good idea in terms of productization. Inspite the fact it's under beta-alpha stages, in general ideas and scope, EU should see only INFO logs. Changed/done.

@candleindark
Copy link
Contributor

candleindark commented Jan 17, 2025

I was able to build and install both the wheel and sdist targets. However, I was not able to run reprostim because the system not being able to locate the zbar shared library. The zbar library was installed though, and I was able to run it in an environment created by Hatch.

Running in the environment created by Hatch
Developer/Dartmouth/reprostim  rf-library ✔                                                                                                                           13d14h
▶ hatch shell
source "/Users/isaac/Library/Application Support/hatch/env/virtual/reprostim/p_0OX6WF/reprostim/bin/activate"

Developer/Dartmouth/reprostim                                                                                                                                         13d14h
▶ source "/Users/isaac/Library/Application Support/hatch/env/virtual/reprostim/p_0OX6WF/reprostim/bin/activate"
(reprostim)
Developer/Dartmouth/reprostim  rf-library ✔                                                                                                                           13d14h
▶ reprostim --help
Usage: reprostim [OPTIONS] COMMAND [ARGS]...

 Command-line interface to run ReproStim tools and services. To see help for
 the specific command, run:

      reprostim COMMAND --help

 e.g. reprostim timesync-stimuli --help

Options:
 --version                       Show the version and exit.
 -l, --log-level [DEBUG|INFO|WARNING|ERROR|CRITICAL]
                                 Set the logging level.
 -f, --log-format TEXT           Set the logging format string. For the
                                 pattern details see standard Python
                                 'logging.Formatter' documentation.
 --help                          Show this message and exit.

Commands:
 detect-noscreen   Utility to determine no-signal frames in...
 echo              Echo the provided message or the default value.
 qr-parse          Utility to parse video and locate integrated QR time...
 timesync-stimuli  PsychoPy reprostim timesync-stimuli script.
(reprostim)
Developer/Dartmouth/reprostim  rf-library ✔                                                                                                                           13d14h
▶ which zbarimag
zbarimag not found
(reprostim)
Developer/Dartmouth/reprostim  rf-library ✔                                                                                                                          13d14h  ⍉
▶ which zbarimg
/opt/homebrew/bin/zbarimg
(reprostim)
Developer/Dartmouth/reprostim  rf-library ✔                                                                                                                           13d14h
▶ zbarimg --version
0.23.93
(reprostim)
Running in an environment created by `venv`
~/Downloads
▶ python -m venv venv && source venv/bin/activate
(venv)
~/Downloads
▶ pip list
Package    Version
---------- -------
pip        22.0.4
setuptools 58.1.0
WARNING: You are using pip version 22.0.4; however, version 24.3.1 is available.
You should consider upgrading via the '/Users/isaac/Downloads/venv/bin/python -m pip install --upgrade pip' command.
(venv)
~/Downloads
▶ pip install -U pip setuptools
Requirement already satisfied: pip in ./venv/lib/python3.9/site-packages (22.0.4)
Collecting pip
  Using cached pip-24.3.1-py3-none-any.whl (1.8 MB)
Requirement already satisfied: setuptools in ./venv/lib/python3.9/site-packages (58.1.0)
Collecting setuptools
  Using cached setuptools-75.8.0-py3-none-any.whl (1.2 MB)
Installing collected packages: setuptools, pip
  Attempting uninstall: setuptools
    Found existing installation: setuptools 58.1.0
    Uninstalling setuptools-58.1.0:
      Successfully uninstalled setuptools-58.1.0
  Attempting uninstall: pip
    Found existing installation: pip 22.0.4
    Uninstalling pip-22.0.4:
      Successfully uninstalled pip-22.0.4
Successfully installed pip-24.3.1 setuptools-75.8.0
(venv)
~/Downloads
▶ pip list
Package    Version
---------- -------
pip        24.3.1
setuptools 75.8.0
(venv)
~/Downloads
▶ pip install /Users/isaac/Downloads/o/reprostim-0.0.6.tar.gz
Processing ./o/reprostim-0.0.6.tar.gz
  Installing build dependencies ... done
  Getting requirements to build wheel ... done
  Preparing metadata (pyproject.toml) ... done
Collecting click-didyoumean>=0.3.1 (from reprostim==0.0.6)
  Using cached click_didyoumean-0.3.1-py3-none-any.whl.metadata (3.9 kB)
Collecting click>=8.1.7 (from reprostim==0.0.6)
  Using cached click-8.1.8-py3-none-any.whl.metadata (2.3 kB)
Collecting numpy>=1.26.4 (from reprostim==0.0.6)
  Using cached numpy-2.0.2-cp39-cp39-macosx_14_0_arm64.whl.metadata (60 kB)
Collecting opencv-python>=4.9.0.80 (from reprostim==0.0.6)
  Using cached opencv_python-4.11.0.86-cp37-abi3-macosx_13_0_arm64.whl.metadata (20 kB)
Collecting pydantic>=2.7.1 (from reprostim==0.0.6)
  Using cached pydantic-2.10.5-py3-none-any.whl.metadata (30 kB)
Collecting pyzbar>=0.1.9 (from reprostim==0.0.6)
  Using cached pyzbar-0.1.9-py2.py3-none-any.whl.metadata (10 kB)
Collecting qrcode>=8.0 (from reprostim==0.0.6)
  Using cached qrcode-8.0-py3-none-any.whl.metadata (17 kB)
Collecting annotated-types>=0.6.0 (from pydantic>=2.7.1->reprostim==0.0.6)
  Using cached annotated_types-0.7.0-py3-none-any.whl.metadata (15 kB)
Collecting pydantic-core==2.27.2 (from pydantic>=2.7.1->reprostim==0.0.6)
  Using cached pydantic_core-2.27.2-cp39-cp39-macosx_11_0_arm64.whl.metadata (6.6 kB)
Collecting typing-extensions>=4.12.2 (from pydantic>=2.7.1->reprostim==0.0.6)
  Using cached typing_extensions-4.12.2-py3-none-any.whl.metadata (3.0 kB)
Using cached click-8.1.8-py3-none-any.whl (98 kB)
Using cached click_didyoumean-0.3.1-py3-none-any.whl (3.6 kB)
Using cached numpy-2.0.2-cp39-cp39-macosx_14_0_arm64.whl (5.3 MB)
Using cached opencv_python-4.11.0.86-cp37-abi3-macosx_13_0_arm64.whl (37.3 MB)
Using cached pydantic-2.10.5-py3-none-any.whl (431 kB)
Using cached pydantic_core-2.27.2-cp39-cp39-macosx_11_0_arm64.whl (1.8 MB)
Using cached pyzbar-0.1.9-py2.py3-none-any.whl (32 kB)
Using cached qrcode-8.0-py3-none-any.whl (45 kB)
Using cached annotated_types-0.7.0-py3-none-any.whl (13 kB)
Using cached typing_extensions-4.12.2-py3-none-any.whl (37 kB)
Building wheels for collected packages: reprostim
  Building wheel for reprostim (pyproject.toml) ... done
  Created wheel for reprostim: filename=reprostim-0.0.6-py2.py3-none-any.whl size=67854 sha256=64fab4f488206429272ea0d76ea76051e35d975bd8c3ecb6b34a364900ab34c0
  Stored in directory: /Users/isaac/Library/Caches/pip/wheels/14/62/93/52fe5e98200160b0caebe0e4c7cf9738a518ab00ddbda45bb2
Successfully built reprostim
Installing collected packages: pyzbar, typing-extensions, qrcode, numpy, click, annotated-types, pydantic-core, opencv-python, click-didyoumean, pydantic, reprostim
Successfully installed annotated-types-0.7.0 click-8.1.8 click-didyoumean-0.3.1 numpy-2.0.2 opencv-python-4.11.0.86 pydantic-2.10.5 pydantic-core-2.27.2 pyzbar-0.1.9 qrcode-8.0 reprostim-0.0.6 typing-extensions-4.12.2
(venv)
~/Downloads
▶ reprostim
Traceback (most recent call last):
  File "/Users/isaac/Downloads/venv/bin/reprostim", line 5, in <module>
    from reprostim.cli.entrypoint import main
  File "/Users/isaac/Downloads/venv/lib/python3.9/site-packages/reprostim/cli/entrypoint.py", line 60, in <module>
    from .cmd_qr_parse import qr_parse  # noqa: E402
  File "/Users/isaac/Downloads/venv/lib/python3.9/site-packages/reprostim/cli/cmd_qr_parse.py", line 10, in <module>
    from ..qr.qr_parse import do_info, do_parse
  File "/Users/isaac/Downloads/venv/lib/python3.9/site-packages/reprostim/qr/qr_parse.py", line 17, in <module>
    from pyzbar.pyzbar import ZBarSymbol, decode
  File "/Users/isaac/Downloads/venv/lib/python3.9/site-packages/pyzbar/pyzbar.py", line 7, in <module>
    from .wrapper import (
  File "/Users/isaac/Downloads/venv/lib/python3.9/site-packages/pyzbar/wrapper.py", line 151, in <module>
    zbar_version = zbar_function(
  File "/Users/isaac/Downloads/venv/lib/python3.9/site-packages/pyzbar/wrapper.py", line 148, in zbar_function
    return prototype((fname, load_libzbar()))
  File "/Users/isaac/Downloads/venv/lib/python3.9/site-packages/pyzbar/wrapper.py", line 127, in load_libzbar
    libzbar, dependencies = zbar_library.load()
  File "/Users/isaac/Downloads/venv/lib/python3.9/site-packages/pyzbar/zbar_library.py", line 65, in load
    raise ImportError('Unable to find zbar shared library')
ImportError: Unable to find zbar shared library
(venv)
~/Downloads                                                                                                                                                                  ⍉
▶ which zbarimg
/opt/homebrew/bin/zbarimg
(venv)
~/Downloads
▶ zbarimg --version
0.23.93

I don't know if this is a MacOS issue. @yarikoptic If you make zbar available on typhon, I can test it there as well.

@yarikoptic
Copy link
Member Author

yarikoptic commented Jan 17, 2025

Two issues.

  1. we should avoid importing all the dependencies if the point is just to get list of commands or --help about them. How to do that

move import of actual functionality into the click's interface function, so we do not import any dependencies for mere --help etc, i.e. looking at current we see that import of do_ functions happen at the top of the file. just move into a function

  File "/Users/isaac/Downloads/venv/lib/python3.9/site-packages/reprostim/cli/cmd_qr_parse.py", line 10, in <module>
    from ..qr.qr_parse import do_info, do_parse
  File "/Users/isaac/Downloads/venv/lib/python3.9/site-packages/reprostim/qr/qr_parse.py", line 17, in <module>
    from pyzbar.pyzbar import ZBarSymbol, decode

I will push some moves performed, please check the rest and adjust

  1. the actual issue with the library not being available: we figured it out -- venv uses systemwide python, which does not have libraries from homebrew available. The hatch one was smarter and found/used homebrew python thus was able to find its libraries... or smth like that. The point is -- we might want to mention that in README.md in installation notes.

edit: also pushed commit to strip double ERROR in msg

…help and avoid crashes

ATM on OSX if libzbar is not available, even running plain `reprostim` would cause a crash.
@candleindark
Copy link
Contributor

2. the actual issue with the library not being available: we figured it out -- venv uses systemwide python, which does not have libraries from homebrew available. The hatch one was smarter and found/used homebrew python thus was able to find its libraries... or smth like that. The point is -- we might want to mention that in README.md in installation notes.

The pyzbar doc directions people to install zbar in MacOS using homebrew. Here are interpreters that I have confirmed that were (were not) able to located the zbar installation by homebrew.

Python interpreters and associated venvs able to locate zbar installed by homebrew
system @ /usr/bin/python3 False
python installed by homebrew True
python installed by pyenv False

@yarikoptic
Copy link
Member Author

@candleindark thanks for review. You can continue discussion or may be even create separate issues. But since @vmdocua already uploaded to pypi https://pypi.org/project/reprostim/ (0.0.7) let's merge and we will tag with 0.0.7 the 1bf01ea

@yarikoptic yarikoptic merged commit 9eff498 into master Jan 22, 2025
3 checks passed
@yarikoptic yarikoptic deleted the rf-library branch January 22, 2025 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants